-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[정성현] sprint9 #123
The head ref may contain hidden characters: "Next-\uC815\uC131\uD604-sprint9"
[정성현] sprint9 #123
Conversation
- 프로젝트 내 불필요 파일 및 코드 제거
- 프로젝트 구조화 및 경로 별칭 적용 - 메타 데이터 작성 및 전역 CSS 적용 - README.md 작성
- 레이아웃 구현 및 App 컴포넌트에 적용 - 헤더 구현 - 관련 페이지 컴포넌트 작성
- api 구조, api 타입, api 훅 구현 - getArticles API 구현
- 미디어 쿼리 적용 - api 적용 - CSS 관련 일부 적용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
성현님 이번 스프린트 미션 고생하셨습니다~
여러모로 신경쓰신 부분이 많이 보였습니다. 고민 많이 하신만큼 잘 작성하셔서 리뷰드릴 것이 많이 없었습니다.
말씀해주신 리렌더링 환경에서의 반응형 데이터 패칭을 대체 어떻게 해야 할 지 갈피가 안잡힙니다ㅠ
요부분 코멘트 남겨드렸어요! 성현님 스스로 해결하실 수 있을것 같아 힌트만 드렸으니 하다가 안되시면 DM 주세요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3:
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3:
👍
<Head> | ||
<title>자유게시판 - 판다마켓</title> | ||
</Head> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3:
👍
useEffect(() => { | ||
setMedia(checkMedia(window.innerWidth)); | ||
|
||
const handleWindowResize = () => { | ||
setMedia(checkMedia(window.innerWidth)); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3:
동일한 코드가 반복되는 것 같아서 아래처럼 하셔도 좋을 것 같습니다.
const handleWindowResize = useCallback(() => {
setMedia(checkMedia(window.innerWidth));
}, []);
useEffect(() => {
handleWindowResize();
window.addEventListener("resize", handleWindowResize);
return () => {
window.removeEventListener("resize", handleWindowResize);
};
}, [handleWindowResize]);
export default function BestItem({ data }: ItemProps) { | ||
const { | ||
id, | ||
title, | ||
writer: { nickname }, | ||
updatedAt, | ||
image, | ||
likeCount, | ||
} = data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2:
이렇게 받으셔야 하는 이유가 있을까요?
다른 props를 받는것도 아니라서 data이라는 객체를 받는것보다 각각의 값을 전달받는게 사용하기에 더 편할 것 같아서요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
부모 컴포넌트(BestItems)쪽에서 data.id, data.title, ... 이런식으로 뿌리는게 너무 길어져서 받은 data 그대로 전달했는데, 강사님 의견이 BestItem 입장에서 속성을 명시하는데에 더 정확하고 나은 것 같습니다!👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2:
처음 실행이 되었을때는 window innerWidth를 읽어와서 크기에 따른 값을 설정해주는 로직이 실행되지 않는다고 말씀해주셨습니다~
코드를 보니 default 값인 PC였다가 곧 window innerWidth에 맞는 값으로 변경이 되는데, 값이 변경된 후 다시 data fetching을 하지 않고 있습니다~
한번 확인해보시고 수정해보시면 좋을 것 같아요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3:
type을 잘 정리해주신 것 좋습니다~
다만 API를 많이 연결하실수록 해당 파일이 커질가능성이 크니 추후에는 관련된 타입끼리 분리하셔도 좋을 것 같아요~
요구사항
배포 URL
기본
심화
주요 변경사항
스크린샷
PC
모바일
멘토에게